Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[김선호] 미션 4 - 취약점 대응 & 리팩터링 리뷰 요청드립니다 🚀 #9

Merged
merged 14 commits into from
Mar 31, 2025

Conversation

haero77
Copy link

@haero77 haero77 commented Mar 18, 2025

재연님 안녕하세요! 믿고 기다려주셔서 감사합니다 ㅎㅎ

사전에 말씀드린대로 우선 3단계까지만 구현하고 리뷰 요청 드립니다.
피드백 반영 및 4단계는 이번 주 주말인 03.22(토)~03.23(일) 중으로 제출하도록 하겠습니다.

이번 미션에서 이런 포인트/인사이트는 반드시 챙겨가면 좋은 점이나, 이 외에 다른 리뷰들도 많이 달아주셔도 너무 좋습니다 ㅎㅎ
궁금한 점은 PR 코멘트로 남겨두겠습니다. 그럼 리뷰 잘 부탁드립니다..! 🙇‍♂️

@haero77 haero77 changed the title Step1 3 [김선호] 미션 4 - 취약점 대응 & 리팩터링 리뷰 요청드립니다 🚀 Mar 18, 2025
Comment on lines +10 to +17
/**
* Server-Side Rendering 시에는 HttpSessionCsrfTokenRepository 사용.
* -> 쿠키에 CSRF 토큰을 저장하지 않고 세션에만 저장하므로 헤더에 CSRF 토큰을 실을 수는 없음. (테스트에서는 헤더에 CSRF 토큰을 실음)
* -> 반면에 SSR에서는 html hidden 필드에 CSRF 토큰을 '_csrf' 파라미터에 실어서 전송할 수 있음.
* Client-Side Rendering 시에는 CookieCsrfTokenRepository 사용.
* -> 쿠키에 CSRF 토큰 값을 저장해야 하므로 프론트에서 쿠키값을 읽어서 헤더에 CSRF 토큰을 실을 수 있음.
*/
public final class HttpSessionCsrfTokenRepository implements CsrfTokenRepository {
Copy link
Author

@haero77 haero77 Mar 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

처음에는 CSRF 필터 구현을 스킵하려 했으나, CSRF 피해 사례를 찾아보니 생각보다 피해 사례도 꽤 되더라고요. 그래서 시간이 조금 더 걸리더라도 이번 기회에 직접 구현해보면서 CSRF 방어에 대한 감을 잡아보고자 공부를 조금 해봤습니다.

그리고 CsrfTokenRepository를 구현할 때 쿠키 저장 방식으로 구현할 지, HTTP 세션 저장 방식으로 구현할지에 대해 조금 고민해봤는데 CSR 방식에서는 쿠키를, SSR 방식에서는 HTTP 세션을 사용하는 것이 낫다고 결론을 내렸는데 근거는 다음과 같습니다.

1. HTTP 세션에 토큰 저장 -> SSR에 적합

타임리프와 같은 템플릿 엔진을 사용하는 경우 토큰값을 직접 프론트로 내려줄 수 있기 때문에,
이 경우 프론트에서는 굳이 번거롭게 쿠키나 세션을 조회하거나 할 필요가 없습니다.
서버에서는 세션 저장소의 토큰값과 쿼리 파라미터로 넘어온 토큰값을 검증하면 되니 인증 방식도 간편합니다.

2. 쿠키에 토큰 저장 -> CSR에 적합

일단 CSR의 경우 쿼리 파라미터나 헤더로 토큰을 실어서 서버로 전송해야하는데, 세션 ID로만 토큰이 관리되면 프론트에서는 이 값을 알 수가 없습니다. 즉 서버의 CSRF 인증을 통과하지 못하게되고, 따라서 쿠키에 CSRF 토큰값을 실어 프론트로 내려준 후, 프론트에서는 헤더에 이 토큰을 실어서 다시 전송하는 식으로 구현할 수 밖에 없습니다. 서버에서는 쿠키의 토큰값과 헤더의 토큰값을 비교하는 방식으로 인증을 처리합니다.

공부 하다가 CSR이든, SSR이든 헤더나 파라미터에서 토큰을 추출해서 검증하지 않고 그냥 쿠키(세션ID)로 토큰을 추출해서 검증하지 않으면 되지 않나? 라고 잠깐 생각이 들었는데, 이렇게 되면 브라우저에서 쿠키가 자동으로 전송됨에 따라 CSRF 방어가 불가능해지니까 결국 헤더나 쿼리 파라미터 등으로 추가적으로 토큰 값을 받아야함을 재차 깨닫게 되었습니다.

제가 생각해본 결론은 위와 같습니다. 제 근거가 적절할지, 또 위 내용과 관련해서 리뷰어님은 어떻게 생각하시는지 궁금하네요 ㅎㅎ 의견 들려주시면 감사하겠습니다..!

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

잘 접근해서 정리해 주셨어요 👍 저도 동일한 의견을 가지고 있어서..! 제가 해당 내용을 고민하며 들었던 추가 질문을 남겨두도록 하겠습니다!

  1. CSR에서 쿠키 기반의 CSRF를 사용할 경우 SameSite 옵션은 어떻게 하는게 좋을까?
  2. 서버에서 별도로 CSRF 토큰을 관리하지 않고 방어할 수 있는 방법은 없을까?
  3. JWT 기반의 인증을 하고 있다면, CSRF 방어 방식이 달라질게 있을까?
  4. CSR 환경에서 CSRF 토큰이 아닌 CORS 정책을 활용해 방어할 수 있지 않을까?

와 같은 고민으로 이어졌던 것 같은데, 선호님도 해당 내용을 고민해 보셨을까요? 😄

Copy link
Author

@haero77 haero77 Mar 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

미처 생각해보지 못한 내용들이네요..! 공부한 내용을 근거로, 제가 생각한 기준을 말씀드려보겠습니다...! 😁
혹시 제가 잘못알고 있는 부분이 있거나 하는 경우 말씀 부탁드릴게요..! (별개로 재연님 의견도 궁금하네요 ㅎㅎ)


1. CSR에서 쿠키 기반의 CSRF를 사용할 경우 SameSite 옵션은 어떻게 하는게 좋을까?

  • Strict 사용 시
    1. 유저가 우리 서비스에 방문 & 로그인 후 CSRF 토큰 발급 및 쿠키에 토큰 저장.
    2. 유저가 다른 사이트(예: google.com)에서 링크 등을 이용해 우리 서비스의 프론트로 이동할 때, origin이 google.com이므로 CSRF 쿠키가 우리 서비스의 프론트로 이동하는 HTTP 요청에 포함되지 않음.
    3. 우리 프론트는 HTTP 요청에서 CSRF 쿠키를 못 받아온 상태이므로 우리 서버로 API 요청 시 헤더에 CSRF 토큰을 실을 수 없음.
    4. 서버에서 CSRF 검증 실패로 403 에러 발생. 유저 경험 하락.
  • Lax 사용 시
    1. 유저가 우리 서비스 방문후 CSRF 쿠키 발급.
    2. 유저가 google.com 등에 있다가 링크 클릭으로 우리 서비스 이동할 경우 origin이 google.com 이지만 sameSite=Lax 이므로 CSRF 쿠키 전송.
    3. 우리 프론트에서 CSRF 쿠키가 있으므로 서버에 정상적인 요청 가능. 유저 경험을 크게 해치지 않으면서 CSRF 방어는 브라우저 레벨에서 유지.
  • None
    1. 모든 요청에 쿠키를 전송하므로, 브라우저 레벨에서 CSRF 방어 불가.

일반적으로 다른 사이트를 통해 서비스에 접속하는 경우가 있을 것으로 예상되어 Lax를 고려해볼 것 같습니다..!

다만 프론트랑 서버를 같은 서브 도메인을 쓰는 경우는 쿠키에 Domain=서브도메인 옵션을 주면 프론트에서 CSRF 쿠키에 접근 가능해지므로, SameSite=Strict로 설정해서 CSRF 공격을 높은 수준으로 방어하면서 UX 저하도 없게끔 구성해볼 것 같습니다

다시 생각해보니 Domain 옵션과는 무관하게 SameSite=Strict 라면 외부 사이트에서 우리 프론트에 접속할 때 애초에 쿠키가 넘어오지 않아 쿠키에 접근이 불가능할테니, 이런 부분에서 일단은 Lax를 사용해볼 것 같습니다..!


2. 서버에서 별도로 CSRF 토큰을 관리하지 않고 방어할 수 있는 방법은 없을까?

만약에 인증을 쿠키(세션)을 이용하여 처리하고 있다면, 해당 인증 쿠키에 SameSite=Lax 또는 SameSite=Strict 옵션을 설정함으로써 서버에서 CSRF 토큰을 관리하는 것과 같은 효과를 가질 것 같습니다. (다른 사이트에서 CSRF 공격이 이루어질 경우 origin이 달라 인증 쿠키가 서비스로 넘어오지 않을테니까요..!)


3. JWT 기반의 인증을 하고 있다면, CSRF 방어 방식이 달라질게 있을까?

JWT 인증을 사용할 경우에, 크게 Authorzation 헤더에 AT를 넣는 경우와 쿠키로 AT를 유지하는 방법이 있을 것 같은데요, 헤더를 쓰냐 쿠키를 쓰냐에 따라 CSRF 방어 방식이 차이가 날 것 같습니다.

먼저 쿠키를 쓰는 경우에는 2번에서 말씀드린 것처럼 CSRF 방어를 위해 SameSite 옵션을 Lax 또는 Strict로 설정합니다.

그런데 만약 쿠키가 아닌 헤더로 인증하게 된다면, accessToken에 대한 CSRF 방어 자체가 불필요해지게 될 것 같아요. (CSRF 방어는 브라우저의 쿠키가 자동으로 전송되는 취약점을 이용한 것이므로) 다만 refreshToken 같은 경우는 여전히 쿠키로 전달해줘야하므로 SameSite 옵션은 여전히 Lax 이상으로 줄 필요가 있겠네요..!


4. CSR 환경에서 CSRF 토큰이 아닌 CORS 정책을 활용해 방어할 수 있지 않을까?

CORS 정책을 활용해 CSRF 방어를 막지는 못할 것 같다고 생각했습니다.
예를 들어 api 서버인 backend.com에서 서비스의 프론트 origin인 https://front.com을 allow origin으로 설정했고, backend.com의 요청에 필요한 쿠키는 SameSite=None 설정되어있다고 가정해보겠습니다.

위 경우 google.com 에서 front.com으로 요청 시 HTTP 요청에 쿠키가 포함되고, 이어서 front.com에서 backend.com으로의 요청에도 쿠키가 포함되므로 CSRF 방어가 어려울 것 같네요..!

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

와우 너무 잘 정리해 주셨어요 💯💯💯

제가 학습이 부족할수도 있지만.. 제가 봤을 때 모든 것을 해결할 수 있는 방법은 없는 것 같아요.
다만 서비스 특성, 인증 구조(JWT, 세션, ...), UX 요구사항 등을 고려해 상황에 맞게 방식을 조합해서 사용하는 것이 가장 현실적인 접근인 것 같아요 ㅎㅎ

추가로 알면 좋을 것 같은 내용 남겨두겠습니다 :)

  • SameSite에만 의존하는 CSRF 방어 방식은 브라우저 지원 정책(예: 구형 브라우저, 일부 iOS 버그 등)에 따라 제한이 존재하기 때문에, 실무에서는 SameSite는 보조 수단이고, CSRF 토큰 기반 방식으로 하는 것이 일반적인 것 같습니다.
  • Refresh Token을 HttpOnly + SameSite=Strict 또는 Lax로 설정하고, access token이 만료되었을 때 클라이언트가 자동 재발급(silent refresh) 하는 방식이 많이 쓰입니다. 이때 일반적으로 인터셉터로 401 응답을 감지해 자동으로 RT를 사용해 재발급 요청할 것 같아요!
  • 위에서 언급한 문제들을 해결하기 위해, CSRF 토큰을 쿠키 + 헤더 기반으로 사용하는 방식도 많이 사용됩니다.
    • 서버가 CSRF 토큰을 JS에서 접근 가능한 쿠키(HttpOnly=false)로 내려주고, 클라이언트는 이 토큰을 꺼내어 요청 헤더에 실어 서버로 전송

Comment on lines +115 to +122
// 인증 후에 인가를 실행해야하므로, 인가 필터를 가장 마지막에 위치
orderedFilters.stream()
.filter(filter -> filter instanceof AuthorizationFilter)
.findFirst()
.ifPresent(filter -> {
orderedFilters.remove(filter);
orderedFilters.add(filter);
});
Copy link
Author

@haero77 haero77 Mar 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

인증/인가 필터가 순서를 지정해주지 않아 SecurityFilterChain의 필터 중 인가 필터가 인증 필터보다 먼저 오는 경우가 생겨 강제적으로 순서를 맞춰주는 식으로 이 문제를 해결해봤습니다. 단순 인증/인가 필터외에도 실제 시큐리티에서는 CORS, CSRF 필터 등 보안 레벨의 필터들도 순서가 명확히 지켜지고 있던데 이런 순서는 절대적인건지 의문점이 들었습니다. 물론 큰 틀에서 보안 -> 인증 -> 인가 필터의 순서는 지켜져야지만, 그 안에서의 순서는 큰 상관이 없는 거 아닌가?라는 그런 의문이 들더라고요. 예를 들면 UsernamePasswordAuthenicationFilterBasicAuthenticationFilter는 크게 순서 상관이 없을 것 같은데 왜 순서가 항상 고정되는가 라는 그런 의문말이죠 ㅎㅎ

관련해서 리뷰어님은 어떻게 생각하시는지 궁금합니다..!

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

예시로 들어주신 UsernamePasswordAuthenicationFilter, BasicAuthenticationFilter 같이 순서가 바뀌어도 문제가 없는 경우도 있지만,
기본적으로 시큐리티는 특성 상 필터의 순서가 굉장히 중요하기 때문에 순서를 지켜주는 방식으로 구현을 한 것 같아요!
내부에서 설정하는 필터인데 어떤 필터는 순서가 없고, 어떤 필터는 있는 것이 오히려 코드의 의문을 준다는 판단이었지 않을까 싶네요 ㅋㅋ
즉, 일관성을 유지하기 위함? 으로 추측해 봅니다 ㅎㅎ

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

위 답변처럼 시큐리티의 필터는 보안 -> 인증 -> 인가를 제외하고도 순서가 중요한 경우가 많을 것 같은데 어떻게 개선해 볼 수 있을까요?

Copy link
Author

@haero77 haero77 Mar 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

시큐리티 필터간 순서 보장 방법

(제가 생각한 방법)

  • 모든 시큐리티 필터들에 대해 필터 클래스와 순서를 1대1로 매칭하는 상수 관리용 클래스 정의
    • 문제점: 신규 시큐리티 필터 추가될 경우 기존 순서를 전부 변경해야하는 번거로움 발생(=유지보수성 낮음)

(실제 시큐리티 구현)

  • FilterOrderRegistration 생성자에서 각 필터 클래스를 각 순서별로 map에 저장. 이후 HttpSecurity에서 FilterOrderRegistration을 참조하여 필터 인스턴스간 순서 조정.
  • 장점: 신규 시큐리티 필터 추가되어도 다른 필터의 순서는 건드리지 않고 해당 필터의 순서만 신경써서 선언하면 되므로 유지보수 간편.

필터간 순서를 어떻게 보장할지 살짝 고민해봤는데, 역시 FilterOrderRegistration 같이 전체 필터에 대해 순서를 관리하는 객체가 하나있는 편이 효율적이라는 생각이 들었습니다!

다만 미션에서는 실제 구현한 필터 수가 그리 많진 않아서, 우선 HttpSecurity 내부에서 강제로 조정하는 방식으로 유지해두었습니다 :)

Comment on lines +23 to +32
public AuthorizeHttpRequestsConfigurer(RoleHierarchy roleHierarchy) {
Assert.notNull(roleHierarchy, "roleHierarchy cannot be null");
this.roleHierarchy = roleHierarchy;
}

@Override
public void init(HttpSecurity http) {
// AuthorizeHttpRequestsConfigurer.hasRole() 보다 HttpSecurity.build()가 먼저 수행되므로,
// init()에서 roleHierarchy 초기화를 진행 시 .hasRole() 시점에 roleHierarchy가 null이 되는 문제 발생 -> 생성자에서 roleHierarchy 주입.
}
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[roleHierarchy 초기화 시점]

AuthorizeHttpRequestsConfigurer::init 에서 roleHierarchy를 초기화 한다면 인가가 제대로 이루어지지 않는 이슈가 있어 생성자에 roleHierarchy를 주입받는 방식으로 해결해봤습니다.

  1. HttpSecurity::buildAuthorizeHttpRequestsConfigurer::hasRole보다 나중에 호출되고,
  2. HttpSecurity::build가 수행되면서 AuthorizeHttpRequestsConfigurer::init이 수행되므로,
  3. 만약 AuthorizeHttpRequestsConfigurer::hasRole을 호출하는 시점에서는 roleHierarchy가 null이므로 이후 제대로 인가가 이루어지지 않는 문제 발생
configurer.hasRole()  -> http.build() -> configurer.init()

그런데, 초기화를 하기위한 init()인데, init()에서 초기화를하지 않고 생성자로 초기화하는 것이 시큐리티 철학에 어울리는지 조금 고민이 되더라고요. 이와 관련해서 리뷰어님은 어떻게 생각하시는지, 또 어떻게 개선해볼 수 있을지 궁금합니다..!

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

먼저 해결하는 방법 자체는 문제가 없을 것 같은데요. 생성자 주입 자체가 시큐리티 철학에 맞냐? 관점에서는 아쉬움이 있는 것 같습니다.
시큐리티의 기본 철학은 최대한 늦게 초기화(Lazy Initialization)하는 것을 선호하기 때문에 조금 개선을 해볼 수 있을 것 같아요 ㅎㅎ

  1. setter를 만든다.
  2. SpringContext의 bean을 가져온다.
  3. 초기화 방식을 변경한다.
http.apply(new RoleHierarchyConfigurer(roleHierarchy))
    .and()
    .authorizeHttpRequests(configurer -> configurer.hasRole("ADMIN"));
  1. 이럴거면 그냥 생성자로 유지한다.

등이 먼저 떠오르네요 ㅎㅎ 추가적인 힌트가 필요하면 말씀해 주세요!

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

재연님께서 말씀해주신 선택지를 검토해본 결과, 지금 구조에서는 AuthorizeHttpRequestsConfigurer::hasRole을 호출하는 시점에 roleHierarchy에 대한 의존성이 필요하므로 일단 '그냥 생성자로 유지'하는 것으로 결정해봤습니다..!

'3. 초기화 방식을 변경'하는 경우에 앱 개발자가 RoleHierarchyConfigurer 인스턴스를 직접 생성해서 설정하기를 기대하기는 조금 어렵지 않을까 라고 생각이 들어서, 여기서도 우선 생성자 주입을 유지해도 괜찮겠다는 생각이 들었습니다.

다만 roleHierarchy를 생성자에서 직접 주입받기 보다는 실제 AuthorizeHttpRequestsConfigurer 처럼 생성자에서는 ApplicationContext만 주입 받고 roleHierarchy 의존성은 직접 context에서 찾는 방식으로 개선해보면 역할 분리 관점에서 좋겠다는 생각이 들었습니다 ㅎㅎ (시큐리티에서 AuthorizeHttpRequestsConfigurer::hasRole 호출 시점에 생성자에서 초기화한 roleHierarchy를 사용하더라고요..!)

Comment on lines +33 to 41
if (authorizationSucceed(decision)) {
chain.doFilter(servletRequest, servletResponse);
return;
}

// 인가에 실패했는데 인증에 실패해서 인가에 실패한 경우
if (authentication == null || !authentication.isAuthenticated()) {
throw new AuthenticationException();
}
Copy link
Author

@haero77 haero77 Mar 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SecurityConfig에서,
아래와 같이 .anyRequest().permitAll()처럼 설정하는 경우 인증 필터에서는 RequestMatcher에 매칭되지 않으므로 인증 필터는 Authentication을 생성하지 않고, 따라서 AuthorizationFilter에서 403이 아닌 401을 던져줘야했습니다.

인가 필터에서 인증 오류인 401을 던지는 것이 자연스러울지 고민되더라고요. 리뷰어님은 여기에 대해 어떻게 생각하시는지 궁금합니다..! (만약 인가 필터에서 403만 던지는게 객체의 책임에 더 어울리는 거라면, 그럼 401은 누가 어디서 던져야할지 많이 고민되서 일단 인가 필터에서 401을 던지도록하고 구현을 마쳤습니다..!)

 .authorizeHttpRequests(authorize -> authorize
              .anyRequest().permitAll()
 )

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

일반적으로 인증 단계에서는 401을 응답하고, 인가 단계에서는 403을 던지는게 자연스러울 것 같아요!
권한을 확인하는 것이 역할인 필터가 인증이 안됐다는 것을 판단하는 것이 어색하기 때문인데요.
인가 단계에서는 인증이 된 사용자만 넘어온다! 와 같은 설계를 하고 선호님이 말씀하신 케이스를 걸러주는 필터를 추가한다면 조금 더 역할이 명확해질 수 있을 것 같은데 어떻게 생각하시나요?

http.addFilterBefore(new AuthenticationCheckFilter(), AuthorizationFilter.class);

Copy link
Author

@haero77 haero77 Mar 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

재연님 말씀처럼 AuthorizationFilter 직전에 '인증이 필요한데 아직 인증이 되지 않은 경우 401 예외를 던지는' AuthenticatoinCheckFilter를 추가함으로써 위 고민이 손쉽게 해결될 것 같습니다..! (이런 방법은 전혀 생각해보지 못했는데 너무 괜찮네요..!)


추가로 그럼 스프링 시큐리티에서는 이렇게 AuthroizationFilter가 수행되기 전에 Authentication이 없는 경우는 어떻게 처리되고 있을지 소스를 분석해봤는데요, AnonymousAuthenticationFilter에서 앞선 인증 필터에서 authentication이 만들어지지 않았다면 authentication를 만들어서 securityContext를 유지하는 것을 확인했습니다..!

Copy link

@jaeyeonling jaeyeonling left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

미션 기간이 끝났음에도 열심히 진행해 주셨네요 💯
질문 주신 내용에 대한 답변과 추가적인 코멘트를 남겨두었으니 확인 부탁드립니다 :)
마지막까지 아자아자! 화이팅!!

Comment on lines +10 to +17
/**
* Server-Side Rendering 시에는 HttpSessionCsrfTokenRepository 사용.
* -> 쿠키에 CSRF 토큰을 저장하지 않고 세션에만 저장하므로 헤더에 CSRF 토큰을 실을 수는 없음. (테스트에서는 헤더에 CSRF 토큰을 실음)
* -> 반면에 SSR에서는 html hidden 필드에 CSRF 토큰을 '_csrf' 파라미터에 실어서 전송할 수 있음.
* Client-Side Rendering 시에는 CookieCsrfTokenRepository 사용.
* -> 쿠키에 CSRF 토큰 값을 저장해야 하므로 프론트에서 쿠키값을 읽어서 헤더에 CSRF 토큰을 실을 수 있음.
*/
public final class HttpSessionCsrfTokenRepository implements CsrfTokenRepository {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

잘 접근해서 정리해 주셨어요 👍 저도 동일한 의견을 가지고 있어서..! 제가 해당 내용을 고민하며 들었던 추가 질문을 남겨두도록 하겠습니다!

  1. CSR에서 쿠키 기반의 CSRF를 사용할 경우 SameSite 옵션은 어떻게 하는게 좋을까?
  2. 서버에서 별도로 CSRF 토큰을 관리하지 않고 방어할 수 있는 방법은 없을까?
  3. JWT 기반의 인증을 하고 있다면, CSRF 방어 방식이 달라질게 있을까?
  4. CSR 환경에서 CSRF 토큰이 아닌 CORS 정책을 활용해 방어할 수 있지 않을까?

와 같은 고민으로 이어졌던 것 같은데, 선호님도 해당 내용을 고민해 보셨을까요? 😄

Comment on lines +115 to +122
// 인증 후에 인가를 실행해야하므로, 인가 필터를 가장 마지막에 위치
orderedFilters.stream()
.filter(filter -> filter instanceof AuthorizationFilter)
.findFirst()
.ifPresent(filter -> {
orderedFilters.remove(filter);
orderedFilters.add(filter);
});

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

예시로 들어주신 UsernamePasswordAuthenicationFilter, BasicAuthenticationFilter 같이 순서가 바뀌어도 문제가 없는 경우도 있지만,
기본적으로 시큐리티는 특성 상 필터의 순서가 굉장히 중요하기 때문에 순서를 지켜주는 방식으로 구현을 한 것 같아요!
내부에서 설정하는 필터인데 어떤 필터는 순서가 없고, 어떤 필터는 있는 것이 오히려 코드의 의문을 준다는 판단이었지 않을까 싶네요 ㅋㅋ
즉, 일관성을 유지하기 위함? 으로 추측해 봅니다 ㅎㅎ

Comment on lines +23 to +32
public AuthorizeHttpRequestsConfigurer(RoleHierarchy roleHierarchy) {
Assert.notNull(roleHierarchy, "roleHierarchy cannot be null");
this.roleHierarchy = roleHierarchy;
}

@Override
public void init(HttpSecurity http) {
// AuthorizeHttpRequestsConfigurer.hasRole() 보다 HttpSecurity.build()가 먼저 수행되므로,
// init()에서 roleHierarchy 초기화를 진행 시 .hasRole() 시점에 roleHierarchy가 null이 되는 문제 발생 -> 생성자에서 roleHierarchy 주입.
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

먼저 해결하는 방법 자체는 문제가 없을 것 같은데요. 생성자 주입 자체가 시큐리티 철학에 맞냐? 관점에서는 아쉬움이 있는 것 같습니다.
시큐리티의 기본 철학은 최대한 늦게 초기화(Lazy Initialization)하는 것을 선호하기 때문에 조금 개선을 해볼 수 있을 것 같아요 ㅎㅎ

  1. setter를 만든다.
  2. SpringContext의 bean을 가져온다.
  3. 초기화 방식을 변경한다.
http.apply(new RoleHierarchyConfigurer(roleHierarchy))
    .and()
    .authorizeHttpRequests(configurer -> configurer.hasRole("ADMIN"));
  1. 이럴거면 그냥 생성자로 유지한다.

등이 먼저 떠오르네요 ㅎㅎ 추가적인 힌트가 필요하면 말씀해 주세요!

Comment on lines +33 to 41
if (authorizationSucceed(decision)) {
chain.doFilter(servletRequest, servletResponse);
return;
}

// 인가에 실패했는데 인증에 실패해서 인가에 실패한 경우
if (authentication == null || !authentication.isAuthenticated()) {
throw new AuthenticationException();
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

일반적으로 인증 단계에서는 401을 응답하고, 인가 단계에서는 403을 던지는게 자연스러울 것 같아요!
권한을 확인하는 것이 역할인 필터가 인증이 안됐다는 것을 판단하는 것이 어색하기 때문인데요.
인가 단계에서는 인증이 된 사용자만 넘어온다! 와 같은 설계를 하고 선호님이 말씀하신 케이스를 걸러주는 필터를 추가한다면 조금 더 역할이 명확해질 수 있을 것 같은데 어떻게 생각하시나요?

http.addFilterBefore(new AuthenticationCheckFilter(), AuthorizationFilter.class);

@@ -1 +1,92 @@
# spring-security
> 미션 4: 취약점 대응 & 리팩토링

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

README 정리 👍

Comment on lines +69 to +74
public SecurityFilterChain securityFilterChain2(HttpSecurity http) {
return http
.authorizeHttpRequests(authorize -> authorize
.requestMatchers("/members").hasRole("ADMIN")
.requestMatchers("/members/me").authenticated()
.anyRequest().permitAll()

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
public SecurityFilterChain securityFilterChain2(HttpSecurity http) {
return http
.authorizeHttpRequests(authorize -> authorize
.requestMatchers("/members").hasRole("ADMIN")
.requestMatchers("/members/me").authenticated()
.anyRequest().permitAll()
public SecurityFilterChain securityFilterChain(HttpSecurity http) {
return http
.authorizeHttpRequests(authorize -> authorize
.requestMatchers("/members").hasRole("ADMIN")
.requestMatchers("/members/me").authenticated()
.anyRequest().permitAll()
.exceptionHandling().authenticationEntryPoint(new CustomAuthenticationEntryPoint())
)

리팩터링의 흔적이 남아있군요 ㅎㅎ
추가로 예외 상황에 따라 어떻게 응답을 할 수 있을까? 라는 고민을 하셨던 만큼 해당 부분이 확장 포인트가 될 수도 있을 것 같네요~

  • 메서드 이름에 2 제거
  • exceptionHandling 내용 추가

Copy link
Author

@haero77 haero77 Mar 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

httpSecurity를 이용한 예외 처리 내용에 더 찾아본 결과 다음의 지식을 얻을 수 있었습니다.

  1. http.exceptionHandling을 이용해서 ExceptionHandlingConfigurer에 대한 커스터마이징이 가능
  2. ExceptionHandlingConfigurer의 authenticationEntryPoint(), accessDeniedHandler()를 이용하여 예외 핸들링 커스터마이징
  3. ExceptionHandlingConfigurer에서 예외처리 필터(ExceptionTranslationFilter)를 초기화하고, ExceptionTranslationFilter에서 authenticationEntryPointaccessDeniedHandler를 이용하여 각각 AuthenticationException, AccessDeniedException이 발생했을 때 예외를 핸들링.

여기까지 공부하다가, 문득 예외처리 필터의 배치 순서에 관해 의문점이 들더군요.

AuthenticationFilter -> ExceptionTranslationFilter -> AuthorizationFilter

FilterOrderRegistration을 확인해보니 위 같은 순서로 필터가 배치되었고, 상식적으로 인증 필터보다 예외처리 필터가 앞에 위치해야 인증 예외를 핸들링할텐데, 실제 배치는 예외처리 필터가 인증 필터 뒤에 위치해있어서 혼란스러웠습니다. 그러다보니 결국 다음과 같은 꼬리물음이 따라오더라고요.

1. 인증 필터는 예외처리 필터 앞에 위치하는데, 그럼 인증 필터에서 발생한 AuthenticationException은 어떻게 처리하지?
2. 예외처리 필터 다음에 인증 필터는 없는데, 그럼 예외처리 필터에서는 인증 예외를 핸들링하는 authenticationEntryPoint가 필요없는 것 아닌가?

물음을 해결하기 위해 시큐리티 소스를 또 뜯어봤고, 다음과 같은 해답을 얻을 수 있었습니다.

1. 인증 필터는 예외처리 필터 앞에 위치하는데, 그럼 인증 필터에서 발생한 AuthenticationException은 어떻게 처리하지?

  • 각 인증 필터에서 직접 예외 처리를 진행한다.
    • UsernamePasswordAuthenticationFilter: failureHandler를 이용하여 인증 예외 핸들링
    • BasicAuthenticationFilter: authenticationEntryPoint를 이용하여 인증 예외 핸들링. authenticationEntryPoint는 HttpBasicConfigurer에서 주입
    • ...

2. 예외처리 필터 다음에 인증 필터는 없는데, 그럼 예외처리 필터에서는 인증 예외를 핸들링하는 authenticationEntryPoint가 필요없는 것 아닌가?

인가 필터에서 인가 예외가 발생했지만, 사실은 그것이 인증이 되지 않아서 발생했던 예외인 경우 인가 예외를 인증 예외의 서브 클래스 InsufficientAuthenticationException으로 변환하고, authenticationEntryPoint를 이용하여 예외를 핸들링하기 때문에 authenticationEntryPoint 주입이 필요하다.


재연님께서 예외 핸들링 관련해서 짚어주신 덕분에, 시큐리티에서의 예외 처리 과정을 깊게 살펴볼 수 있었습니다. (심지어 앞 코멘트에서 시큐리티 필터간 순서를 어떻게 보장할지에 대해 FilterOrderRegistration을 공부하는 부분이 있었는데 이 부분과 예외처리 필터의 순서가 같이 엮이면서 진짜 재밌게 공부했습니다.. 🙇‍♂️)

짚어주신 덕분에 말씀처럼 사고가 쭉 확장되어서 앞으로 스프링 시큐리티를 쓸 때 예외 핸들링 관련해서 무리없이 잘 구현해볼 수 있을 것 같네요..! 감사합니다 :-)


import jakarta.servlet.http.HttpServletResponse;

public class AccessDeniedHandler {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

해당 클래스를 분리해주신 이유가 있을까요?

Copy link
Author

@haero77 haero77 Mar 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

인가 예외가 발생할 때마다 403을 내려주도록 하는 것이 번거로워서, 이 로직을 객체로 분리해봤습니다.
다만 현재는 CsrfFilter::doFilterInternal 에서만 사용되고 있어서 이 때는 별도로 객체로 분리하는 것보다 바로 403을 내려줘도 괜찮았을 것 같네요..! (이 부분을 의도해서 질문주신 건지 궁금합니다..!)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

말씀해 주신 부분이 맞습니다!

제 의식의 흐름이

  1. 다른 곳에서도 사용되나? (중복 제거) -> 아니네
  2. 그럼 AccessDeniedHandler가 인터페이스인가? (확장성 고려) -> 아니네
  3. 그럼 역할의 분리를 위한건가? -> 질문해보자!

으로 흘러간 다음 어떤 의도를 담으셨는지 궁금해서 남겼습니다 ㅎㅎ

Comment on lines +115 to +122
// 인증 후에 인가를 실행해야하므로, 인가 필터를 가장 마지막에 위치
orderedFilters.stream()
.filter(filter -> filter instanceof AuthorizationFilter)
.findFirst()
.ifPresent(filter -> {
orderedFilters.remove(filter);
orderedFilters.add(filter);
});

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

위 답변처럼 시큐리티의 필터는 보안 -> 인증 -> 인가를 제외하고도 순서가 중요한 경우가 많을 것 같은데 어떻게 개선해 볼 수 있을까요?

public class HttpSecurityConfiguration {

@Bean
HttpSecurity httpSecurity(

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

HttpSecurity는 보안 설정을 구성하는 객체이며, 시큐리티는 여러 설정을 적용할 수 있도록 설계되어 있습니다.
하지만 HttpSecurity을 Bean으로 등록하면 별도로 추가적인 설정을 적용하기 어려운 문제가 발생할 수 있다고 생각하는데 어떻게 생각하시나요?

Copy link
Author

@haero77 haero77 Mar 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

말씀하신 부분을 고민해봤는데, 확실히 HttpSecurity가 싱글톤 인스턴스로 유지될 경우 앱 개발자가 SecurityFilterChain을 새로 만드는 순간 이전에 설정한 HttpSecurity 설정이 유지되는 문제가 있더라고요.

그래서 이 경우 HttpSecurity를 빈으로 등록할 때, 빈 스코프를 프로토타입으로 변경하여, 빈을 주입할 때마다 httpSecurity를 호출하게 만들어 ApplicationContext 주입은 유지하고, 여러 SecurityFilterChain 빈에서 각각 독립적인 HttpSecurity 설정을 할 수 있게 변경해봤습니다..! 1dfcc3f

private List<RequestMatcher> ignoredCsrfProtectionMatchers = new ArrayList<>();

@Override
public void init(HttpSecurity http) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

init과 configure는 어떤 목적으로 나눠서 사용하고 계신가요?

Copy link
Author

@haero77 haero77 Mar 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

기존에는 단순히 말 그대로 필터 초기화와 필터 설정을 하는 정도로만 인식했는데, 제가 구현한 소스를 보니 명확한 기준이 없이 작성되었음을 인지했습니다. 그래서 필터 초기화 시 필요한 의존성을 기준으로 다음과 같은 기준을 다시 세워봤고, 이것을 61da879에 반영해봤습니다..!

  • init: 필터 설정에 필요한 의존성 주입
  • configure: init 단계에서 준비된 객체를 이용하여 필터를 설정하고, 필터 추가

haero77 added 4 commits March 26, 2025 20:42
- init: 초기 설정, 준비 작업. 필터 설정에 필요한 의존성 주입
- configure: init 단계에서 준비된 객체를 이용하여 필터를 설정하고, 필터 추가
- HttpSecurity가 싱글톤 인스턴스로 유지될 경우 앱 개발자가 SecurityFilterChain을 새로 만드는 순간 이전에 설정한 HttpSecurity 설정이 유지되는 문제가 있음.
- 빈 스코프를 프로토타입으로 변경하여, 빈을 주입할 때마다 httpSecurity를 호출하게 만들어 ApplicationContext 주입은 유지.
- 최종적으로 여러 SecurityFilterChain 빈에서 각각 독립적인 HttpSecurity 설정을 할 수 있게된다.
@haero77
Copy link
Author

haero77 commented Mar 29, 2025

@jaeyeonling 재연님 믿고 기다려주셔서 감사합니다..!
지난 리뷰 이후 추가로 작업한 범위는 여기이고, 추가로 코멘트 남겨주신 부분에 대해 전부 답글을 달아두었습니다 :)

남겨주신 코멘트 덕분에 진짜 공부 많이, 또 깊게 해볼 수 있었습니다. 감사합니다.
그럼 마지막 리뷰 잘 부탁드릴게요..! 🙇‍♂️

Copy link

@jaeyeonling jaeyeonling left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

마지막 미션까지 너무 훌륭하게 구현해 주셨고, 학습에도 열정적으로 임해주셔서 정말 인상깊네요 💯💯
저 역시 큰 도움이 되었고, 즐겁게 리뷰할 수 있었습니다 ㅎㅎ
피드백을 잘 반영해 주셔서 이만 머지하겠습니다!
추가로 제 의견을 남겨두었으니 확인 부탁드리고, 궁금한 점이 있다면 언제든지 편하게 DM 주세요 :)
마지막까지 정말 고생 많으셨습니다~!

Comment on lines +10 to +17
/**
* Server-Side Rendering 시에는 HttpSessionCsrfTokenRepository 사용.
* -> 쿠키에 CSRF 토큰을 저장하지 않고 세션에만 저장하므로 헤더에 CSRF 토큰을 실을 수는 없음. (테스트에서는 헤더에 CSRF 토큰을 실음)
* -> 반면에 SSR에서는 html hidden 필드에 CSRF 토큰을 '_csrf' 파라미터에 실어서 전송할 수 있음.
* Client-Side Rendering 시에는 CookieCsrfTokenRepository 사용.
* -> 쿠키에 CSRF 토큰 값을 저장해야 하므로 프론트에서 쿠키값을 읽어서 헤더에 CSRF 토큰을 실을 수 있음.
*/
public final class HttpSessionCsrfTokenRepository implements CsrfTokenRepository {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

와우 너무 잘 정리해 주셨어요 💯💯💯

제가 학습이 부족할수도 있지만.. 제가 봤을 때 모든 것을 해결할 수 있는 방법은 없는 것 같아요.
다만 서비스 특성, 인증 구조(JWT, 세션, ...), UX 요구사항 등을 고려해 상황에 맞게 방식을 조합해서 사용하는 것이 가장 현실적인 접근인 것 같아요 ㅎㅎ

추가로 알면 좋을 것 같은 내용 남겨두겠습니다 :)

  • SameSite에만 의존하는 CSRF 방어 방식은 브라우저 지원 정책(예: 구형 브라우저, 일부 iOS 버그 등)에 따라 제한이 존재하기 때문에, 실무에서는 SameSite는 보조 수단이고, CSRF 토큰 기반 방식으로 하는 것이 일반적인 것 같습니다.
  • Refresh Token을 HttpOnly + SameSite=Strict 또는 Lax로 설정하고, access token이 만료되었을 때 클라이언트가 자동 재발급(silent refresh) 하는 방식이 많이 쓰입니다. 이때 일반적으로 인터셉터로 401 응답을 감지해 자동으로 RT를 사용해 재발급 요청할 것 같아요!
  • 위에서 언급한 문제들을 해결하기 위해, CSRF 토큰을 쿠키 + 헤더 기반으로 사용하는 방식도 많이 사용됩니다.
    • 서버가 CSRF 토큰을 JS에서 접근 가능한 쿠키(HttpOnly=false)로 내려주고, 클라이언트는 이 토큰을 꺼내어 요청 헤더에 실어 서버로 전송


import jakarta.servlet.http.HttpServletResponse;

public class AccessDeniedHandler {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

말씀해 주신 부분이 맞습니다!

제 의식의 흐름이

  1. 다른 곳에서도 사용되나? (중복 제거) -> 아니네
  2. 그럼 AccessDeniedHandler가 인터페이스인가? (확장성 고려) -> 아니네
  3. 그럼 역할의 분리를 위한건가? -> 질문해보자!

으로 흘러간 다음 어떤 의도를 담으셨는지 궁금해서 남겼습니다 ㅎㅎ

@@ -76,6 +90,19 @@ public SecurityFilterChain securityFilterChain(HttpSecurity http) throws Excepti
- [x] 사용자가 SecurityFilterChain을 정의한 경우 기본 SecurityFilterChain이 설정된다.
- [x] 사용자가 SecurityFilterChain을 정의하지 않은 경우 기본 SecurityFilterChain이 설정되지 않는다.

### 미션을 진행하며 생긴 고민에 대한 흔적 & 피드백 내용

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

README 정리까지.. 너무 훌륭하네요 👍

@jaeyeonling jaeyeonling merged commit 89a6a0e into next-step:haero77 Mar 31, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants